Summary

Adds support for the TUNIT_DISABLE_LOGO environment variable to disable the ASCII art logo, alongside comprehensive environment variables documentation.

Critical Issues

None found ✅

Suggestions

1. Environment Variable Value Checking Pattern Inconsistency

The implementation in BannerCapability.cs:26 uses:

Environment.GetEnvironmentVariable("TUNIT_DISABLE_LOGO") is not null

However, this differs from the more robust pattern used in TestScheduler.cs for TUNIT_MAX_PARALLEL_TESTS, which explicitly parses values. For boolean-style flags, the codebase uses a simpler "is not null" check (as seen in GitHubReporter.cs:27-28), which is correct and consistent.

However, consider documenting in environment-variables.md that any non-null value enables the flag (not just "true"). For example, both of these would disable the logo:

export TUNIT_DISABLE_LOGO=true
export TUNIT_DISABLE_LOGO=1
export TUNIT_DISABLE_LOGO=false  # This would ALSO disable the logo!

The documentation currently shows only =true which might mislead users into thinking =false would enable it. Consider clarifying:

# Set to any value to disable the logo (presence of variable disables it)
export TUNIT_DISABLE_LOGO=1

2. Priority Order Clarification

The documentation states command-line arguments take precedence, but the current implementation in BannerCapability.cs:25-26 checks both conditions with `||":

if (commandLineOptions.IsOptionSet(DisableLogoCommandProvider.DisableLogo)
    || Environment.GetEnvironmentVariable("TUNIT_DISABLE_LOGO") is not null

This means if either is set, the logo is disabled - which is the correct behavior. The documentation correctly describes this, so no code change needed.

3. Minor Documentation Consistency

In environment-variables.md, line 21, the documentation shows:

**Equivalent to:** `--disable-logo`

Consider adding similar "Equivalent to:" sections for all other environment variables that have command-line equivalents (like TUNIT_MAX_PARALLEL_TESTS), which you do on line 82. Good consistency there!

Verdict

✅ APPROVE - No critical issues. The implementation follows existing patterns in the codebase. The suggestions are minor documentation clarifications that could be addressed in a follow-up PR.